Skip to content

chore(auth): legacy credential provider to use AuthOutputs instead of AmplifyConfig types #5303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

NikaHsn
Copy link
Contributor

@NikaHsn NikaHsn commented Aug 15, 2024

Issue #, if available:

Description of changes:

  • legacy credential provider to use AuthOutputs instead of AmplifyConfig types
  • legacy ios cognito keys to have an String field instead of CognitoUserPoolConfig
  • credential store state machine to use AuthOutputs instead of AuthConfiguration

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@NikaHsn NikaHsn requested a review from a team as a code owner August 15, 2024 20:42
Equartey
Equartey previously approved these changes Aug 22, 2024
@NikaHsn NikaHsn force-pushed the chore/auth/legacy-credential-provider branch from ecd7a0c to 90a7f7e Compare August 29, 2024 20:43
String username,
AuthOutputs authOutputs,
) async {
if (authOutputs.userPoolClientId == null) return null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Remove the force casts. If you create variables for userPoolId and userPoolClientId and return early if they are null, then Dart will not require the force case.

    final userPoolId = authOutputs.userPoolId;
    final userPoolClientId = authOutputs.userPoolClientId;
    if (userPoolId == null || userPoolClientId == null) return null;
    final userPoolStorage = await _getUserPoolStorage();
    final cognitoUserKeys = LegacyCognitoUserKeys(userPoolClientId);
    final currentUserId = await userPoolStorage.read(
      key: cognitoUserKeys[LegacyCognitoKey.currentUser],
    );
    if (currentUserId == null) return null;
    final keys = LegacyDeviceSecretKeys(currentUserId, userPoolId);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This applies to several other locations in the PR.

if (currentUserId == null) return;
final keys = LegacyDeviceSecretKeys(currentUserId, userPoolConfig);
final asfKeys = LegacyAsfDeviceKeys(currentUserId, userPoolConfig);
if (currentUserId == null || authOutputs.userPoolId == null) return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same suggestion as above. Remove the force cast by creating a userPoolId var.

final currentUser = await userPoolStorage.read(
key: cognitoUserKeys[LegacyCognitoKey.currentUser],
);
if (currentUser != null) {
final userPoolKeys = LegacyCognitoUserPoolKeys(
currentUser,
userPoolConfig,
authOutputs.userPoolClientId!,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See suggestion below about remove force casts

@NikaHsn NikaHsn force-pushed the chore/auth/legacy-credential-provider branch from a09c772 to de14ec5 Compare September 11, 2024 22:52
@NikaHsn NikaHsn force-pushed the chore/auth/legacy-credential-provider branch from de14ec5 to 1ea7f5f Compare September 11, 2024 23:08
@NikaHsn NikaHsn merged commit 51d01c4 into main Sep 16, 2024
77 checks passed
Equartey pushed a commit that referenced this pull request Oct 21, 2024
Equartey pushed a commit that referenced this pull request Oct 21, 2024
Equartey pushed a commit that referenced this pull request Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants